fix(graph-ui): fix Windows folder navigation in the index picker#739
Conversation
rarepops
force-pushed
the
fix/windows-folder-picker
branch
from
89795df to
00ce349
Compare
DeusData
added
bug
|
Thanks for the Windows picker fix. Triage: this is a Windows graph UI indexing bug, priority high, because drive/root navigation can block indexing from the UI. For review, we will focus on drive-root breadcrumb targets, typed-path refresh, |
|
Thank you — the Windows navigation work is exactly what remained of #548 after the backend drive-enumeration landed: drive-aware breadcrumbs, typed-drive refresh, roots handling, and the POSIX-negative test is appreciated. One structural ask: commit 61884ca removes the 'Project name' field entirely — that's a cross-platform feature removal and an API-shape/product decision (the backend still accepts project_name), and it sits in a PR whose body says the changes are Windows-scoped. Your rationale for removing it is genuinely reasonable, but it deserves its own PR where it can be discussed as a product question rather than riding along. Could you split that commit out and keep the four navigation commits here? Small heads-up: you may need to rebase the test-file import line once the indexing-error PR lands (it touches the same line). With the split, this merges quickly — thanks again! Update: to get the navigation fixes into the next release, if the split doesn't happen in the next couple of days we'll cherry-pick the four navigation commits ourselves (your authorship preserved on each) and leave the project-name-field question to its own discussion. Whichever is easier for you! |
|
@DeusData sure no problem! I didn't think it was needed honestly, would be good to have a rename for a project or have the name different from the path etc. I will make the change :) |
rarepops
force-pushed
the
fix/windows-folder-picker
branch
from
00ce349 to
28e151a
Compare
|
Heads-up: the |
rarepops
force-pushed
the
fix/windows-folder-picker
branch
from
28e151a to
2b84c1f
Compare
|
Thank you for the quick split — exactly as hoped, and the four navigation commits here are the ones our review greenlit. One last mechanical step: the branch now conflicts with main on the StatsTab test file (the indexing-error surfacing landed yesterday and touched the same import line). A quick rebase/merge of main and this is ready to merge. The product question about the Project-name field continues in #805 on its own merits. Thanks again! |
The index file picker built breadcrumb targets as '/' + segments, so on a Windows drive path (C:/Users/rap) clicking a crumb browsed to '/C:/...', which the backend rejected as 'not a directory'. Only the '.. (up)' button worked. Build drive-aware crumb targets (C:/, C:/Users) and drop the bogus unified '/' root crumb on Windows drive paths; POSIX behavior is unchanged. Add a regression test for Windows breadcrumb navigation and cleanup() for test isolation. Signed-off-by: Zadak <rarepops@protonmail.com>
Typing a path into the Repository path field updated currentPath (and thus the breadcrumb) but never re-fetched the directory listing, so switching drives by typing (e.g. 'D:/') left the previous location's folders showing. This is the only way to change drives when the backend does not enumerate drive roots. Debounce a silent /api/browse refresh when the typed path changes (keeping the last good listing instead of flashing errors mid-typing), and navigate immediately on Enter. Add a regression test. Signed-off-by: Zadak <rarepops@protonmail.com>
…stead On Windows the POSIX '/' quick-jump root is meaningless (browsing it returns an empty listing), yet the picker showed a '/' button whenever the backend did not enumerate drives (older builds return roots=['/']). Clicking it stranded the user on an empty view. Derive Windows-aware quick-jump roots: drop non-drive roots and always include the current drive (parsed from the browsed path), so the button lists the drive root. Other drives remain reachable by typing. Add a regression test. Signed-off-by: Zadak <rarepops@protonmail.com>
The debounced folder-list refresh (and Enter-to-navigate) on the Repository path field now only fires for Windows drive paths (e.g. 'D:/'), where typing is the mechanism for switching drives. POSIX path navigation is left exactly as before. Add a regression test asserting a typed POSIX path does not trigger a re-browse. Signed-off-by: Zadak <rarepops@protonmail.com>
rarepops
force-pushed
the
fix/windows-folder-picker
branch
from
2b84c1f to
d72a84f
Compare
|
Thank you — and thanks for the fast turnaround on both the split and the rebase. The four navigation fixes are exactly what remained of the Windows picker work: drive-aware breadcrumbs, typed-path refresh (correctly scoped to drive paths), and the root handling, with the POSIX-negative test giving confidence nothing changes off-Windows. Merging. The Project-name discussion concluded as keep-but-relabel (landing separately, credited to you) — thanks for pushing that conversation with #805. |
2 participants
Summary
Fixes several issues with the repository folder picker (the New Index modal) on Windows. All changes are frontend-only (
graph-ui) and Windows-scoped, so POSIX (Linux/macOS) navigation is left byte-for-byte unchanged, guarded by anisWinPathcheck.On Windows the picker was effectively unusable: clicking breadcrumb segments errored with "not a directory", switching drives didn't update the listing, and the
/quick-jump button led to an empty view.Changes
C:/Users/x) the breadcrumb built click targets as"/" + segments(/C:/Users), which the backend rejects as "not a directory". It now builds drive-aware targets (C:/,C:/Users) and drops the meaningless leading/crumb on Windows.D:/) updated the input and breadcrumb but never re-fetched the directory listing, so switching drives by typing showed the previous drive's folders. Added a debounced, silent/api/browserefresh (plus Enter-to-navigate), scoped to Windows drive paths. This matters because older backends don't enumerate drives, making typing the only way to switch drives./quick-jump root is meaningless (browsing it returns an empty listing). It now offers drive roots instead, always including the current drive (derived from the browsed path), so it works even when the backend returnsroots: ["/"].Testing
graph-uiunit tests: 12/12 passing, including new regression tests for Windows breadcrumb navigation, typed-drive refresh,/to drive-root replacement, and a POSIX-negative test asserting a typed POSIX path does not trigger a re-browse.D:/, and the drive-root quick-jump all list the correct folders.Notes
src/ui/http_server.c) is unchanged; these fixes work against existing/older builds that returnroots: ["/"].